Reduce data copies in connection receive path#645
Open
mykaul wants to merge 3 commits intoscylladb:masterfrom
Open
Reduce data copies in connection receive path#645mykaul wants to merge 3 commits intoscylladb:masterfrom
mykaul wants to merge 3 commits intoscylladb:masterfrom
Conversation
Optimize buffer management in `_ConnectionIOBuffer` to avoid unnecessary byte
allocations during high-throughput reads.
1. **Buffer Compaction**: Replaced `io.BytesIO(buffer.read())` with a
`getbuffer()` slice. The previous method `read()` allocated a new `bytes`
object for the remaining content before creating the new generic `BytesIO`.
The new approach uses a zero-copy memoryview slice for initialization.
2. **Header Peeking**: Replaced `getvalue()` in `_read_frame_header` with
`getbuffer()`. This allows inspecting the protocol version and frame length
without materializing the entire buffer contents into a new `bytes` string.
Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
There was a problem hiding this comment.
Pull request overview
This PR optimizes buffer management in the connection receive path to reduce memory allocations during high-throughput reads. The changes leverage Python's memoryview capabilities to avoid unnecessary byte copies in two critical hot-path operations.
Key Changes:
- Introduced a
_reset_bufferstatic method that usesgetbuffer()slicing instead ofread()for buffer compaction - Replaced
getvalue()withgetbuffer()in_read_frame_headerfor zero-copy header inspection
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace BytesIO.read() with direct buffer slicing to eliminate one intermediate bytes allocation per received message frame. Changes: - Use getbuffer() to get memoryview of underlying buffer - Slice directly at [body_offset:end_pos] instead of seek+read - Convert memoryview slice to bytes in single operation - Maintain buffer position tracking for proper reset behavior Benefits: - Eliminates one full-frame allocation on hot receive path - Maintains compatibility with existing protocol decoder The memoryview is immediately converted to bytes and released, preventing buffer resize issues while still gaining the allocation savings. Signed-off-by: Yaniv Kaul <ykaul@scylladb.com>
Introduce a lightweight BytesReader class that provides the same read() interface as io.BytesIO but operates directly on the input bytes/memoryview without internal buffering overhead. Changes: - Add BytesReader class with __slots__ for memory efficiency - Replace io.BytesIO(body) with BytesReader(body) in decode_message() - BytesReader.read() returns slices directly, converting memoryview to bytes only when necessary for compatibility Benefits: - Eliminates BytesIO's internal buffer allocation and management - Reduces memory overhead for protocol message decoding - Works seamlessly with both bytes and memoryview inputs - Maintains full API compatibility with existing read_* functions The BytesReader is a minimal implementation focused on the read() method needed by the protocol decoder. It avoids the overhead of io.BytesIO's full file-like interface. Signed-off-by: Yaniv Kaul <ykaul@scylladb.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Optimize buffer management in
_ConnectionIOBufferto avoid unnecessary byte allocations during high-throughput reads.Buffer Compaction: Replaced
io.BytesIO(buffer.read())with agetbuffer()slice. The previous methodread()allocated a newbytesobject for the remaining content before creating the new genericBytesIO. The new approach uses a zero-copy memoryview slice for initialization.Header Peeking: Replaced
getvalue()in_read_frame_headerwithgetbuffer(). This allows inspecting the protocol version and frame length without materializing the entire buffer contents into a newbytesstring.Pre-review checklist
./docs/source/.Fixes:annotations to PR description.